Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: destructured constructor #22

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

pwmcintyre
Copy link
Owner

@pwmcintyre pwmcintyre commented May 19, 2023

This proposal destructures the Logger constructor so that you don't have to supply all parameters, and the order doesn't matter.

For example, if you want to supply a custom Stamper ...

Previous

Since it is currently the last of the optional parameters, you need to supply all the other parameters, there is no option to leave them blank or as default:

const logger = new Logger(LogLevel.DEBUG, StdOutWriter, JSONSerializer, [CustomStamper])

Proposed

Now you can omit parameters you want to leave as defaults:

const logger = new Logger({ stampers: [CustomStamper] })

Comment on lines +48 to +50
constructor(params: Parameters) {
this.options = { ...DefaultOptions, ...params }
}
Copy link
Owner Author

@pwmcintyre pwmcintyre May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could optionally be a non-breaking change by checking the parameter type ... but it gets pretty ugly, i'd rather break the API 🤔

Suggested change
constructor(params: Parameters) {
this.options = { ...DefaultOptions, ...params }
}
constructor(
params?: LogLevel | Options,
write?: Writer,
serialize?: Serializer,
stamps?: Stamper[],
) {
if ( params === undefined ) {
this.options = { ...DefaultOptions }
} else if ( params === undefined && params as LogLevel in LogLevel ) {
this.options = { ...DefaultOptions }
this.options.level = params as LogLevel
this.options.write = write ? write : DefaultOptions.write
this.options.serialize = serialize ? serialize : DefaultOptions.serialize
this.options.stamps = stamps ? stamps : DefaultOptions.stamps
} else {
this.options = { ...DefaultOptions, ...params as Options }
}
}

@pwmcintyre pwmcintyre self-assigned this May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants